Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use 'git clone' instead of 'go get' for the build or developer's … #671

Closed
wants to merge 1 commit into from

Conversation

chavey
Copy link

@chavey chavey commented Nov 20, 2021

Use 'git clone' instead of 'go get' for the build or developer's workflow documentation.

As golang released their new version 1.17.1 go get is not supported
anymore. golang/go#31529 (comment)
suggests using 'git clone' rather than 'go get'.

…low documentation.

As golang released their new version 1.17.1 go get is not supported
anymore. golang/go#31529 (comment)
suggests using 'git clone' rather than 'go get'.
@chavey chavey changed the title Use 'git clone' instead of 'go get' for in the build or developer's … Use 'git clone' instead of 'go get' for the build or developer's … Nov 20, 2021
@@ -45,7 +45,8 @@ automatically set in all console sessions.
To get the source code, run

```
go get github.com/google/pprof
eval {'mkdir -p',cd}\ $GOPATH/src/github.com/google\;
Copy link
Collaborator

@aalexand aalexand Nov 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"eval" seems scary, as well as creating a directory for people. This will also fail if $GOPATH is not set since this will try to create a directory at the root ("/src/github.com/google").

It also creates impression that the source directory has to be under $GOPATH, but $GOPATH is considered largely deprecated since Go 1.13 (I believe its only intended usage is being able to override where the module cache is stored, but that should be rare / not needed).

So, I think I would instead use

cd /tmp
git clone [email protected]:google/pprof.git
cd pprof

and change cd $GOPATH/src/github.com/google/pprof to cd /tmp/pprof in other commands below. This should make it clear that the checkout directory can be in any location and people who read the instruction will override it with their favorite path (e.g. I use ~/src/pprof).

Copy link
Author

@chavey chavey Nov 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want to replace:

"""
Make sure GOPATH is set in your current shell. The common way is to have
something like export GOPATH=$HOME/gocode in your .bashrc file so that it's
automatically set in all console sessions.
"""

with

"""
Make sure to use a GoLang version 1.13 or above.
"""

Copy link
Author

@chavey chavey Nov 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to keep:

"""
When you wish to work with your own fork of the source (which is required to be able to create a pull request), you'll want to get your fork repo as another Git remote in the same github.com/google/pprof directory. Otherwise, if you'll go get your fork directly, you'll be getting errors like use of internal package not allowed when running tests. To set up the remote do something like

cd $GOPATH/src/github.com/google/pprof
git remote add aalexand [email protected]:aalexand/pprof.git
git fetch aalexand
git checkout -b my-new-feature
# hack hack hack
go test -v ./...
git commit -a -m "Add new feature."
git push aalexand

where aalexand is your GitHub user ID. Then proceed to the GitHub UI to send a
code review.
"""

We could replace all the above by:

"""
To make changes to the code:

cd /tmp/pprof
git fetch origin
git checkout -b my-new-feature
# hack hack hack
go test -v ./...
git commit -a -m "Add a new feature."
git push origin

Then proceed to the GitHub UI to send a
code review.
"""

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to say "Make sure to use a GoLang version 1.13 or above" because we already reference https://golang.org/doc/devel/release#policy which as of now means that pprof only supports Go 1.17 and Go 1.16.

@aalexand
Copy link
Collaborator

@chavey I propose this change #673

@chavey chavey closed this Nov 22, 2021
@chavey chavey deleted the chavey-648 branch October 15, 2022 05:04
@aalexand aalexand added type: feat Buganizer type - Feature Request and removed enhancement labels May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes type: feat Buganizer type - Feature Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CONTRIBUTING.md: Instructions to get the source code should be updated for new version of Go
2 participants